-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: improve playlist counts #29381
base: master
Are you sure you want to change the base?
Conversation
pauldambra
commented
Feb 28, 2025
•
edited
Loading
edited
- make the out put a little clearer for debugging since this isn't released yet
- also include whether the value increased
- and how many are unwatched
- refactor the data we're passing around a little
- improve the logging of errors so we can see the failing filters/queries to use as test cases
Size Change: 0 B Total Size: 9.73 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
id_list = count_data.get("session_ids", None) | ||
recordings_counts["query_count"] = len(id_list) if id_list else 0 | ||
recordings_counts["has_more"] = count_data.get("has_more", False) | ||
id_list: list[str] = count_data.get("session_ids", None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
messy, but wrapped in a try/catch and only here so we can validate behind a flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR enhances the session recording playlist counts feature with more detailed information and improved debugging capabilities.
- Added
recordings_counts
structure infrontend/src/types.ts
with separate interfaces for collection and saved filters counts, including watched counts and increase indicators - Updated mock data in
recording_playlists.ts
to include the new count structure with watched/unwatched information - Enhanced tooltip display in
SavedSessionRecordingPlaylists.tsx
with tabular format showing both total and unwatched counts - Renamed
_current_user_viewed
tocurrent_user_viewed
insession_recording_api.py
to make it publicly accessible - Improved error logging in
recordings_that_match_playlist_filters.py
by adding playlist short IDs and filter information - Added visual indicator (up arrow) when saved filters count has increased
9 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
ee/session_recordings/playlist_counters/recordings_that_match_playlist_filters.py
Show resolved
Hide resolved
ee/session_recordings/playlist_counters/recordings_that_match_playlist_filters.py
Show resolved
Hide resolved
ee/session_recordings/playlist_counters/recordings_that_match_playlist_filters.py
Show resolved
Hide resolved
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
"filters": playlist.filters if playlist else None, | ||
"query": query, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
most important change here is grabbing these failing filters/queries so we can figure out the transformations the FE is doing that the BE is missing
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, there are many that didn't calculate and were not picked up again